Skip to content

DLQ CHASM pure task if valid after execution, add unit test verifications to test framework#10502

Open
awln-temporal wants to merge 4 commits into
mainfrom
awln/chasm-pure-task-invalidates
Open

DLQ CHASM pure task if valid after execution, add unit test verifications to test framework#10502
awln-temporal wants to merge 4 commits into
mainfrom
awln/chasm-pure-task-invalidates

Conversation

@awln-temporal

Copy link
Copy Markdown
Contributor

What changed?

If pure task is still valid after execution, we throw a new task error and move the task to DLQ to avoid stuck executions. Also adds unit test verifications to the framework.

Why?

Task Validation checks if a task should be executed by running a Predicate on the component state. Pure tasks should not be valid after execution, but as of now, there is no such verifications in place to prevent a task from remaining valid after execution, leading to stuck executions. The logical task remains in the CHASM tree, and no other physical task is generated.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@awln-temporal awln-temporal requested review from a team as code owners June 3, 2026 16:31
@awln-temporal awln-temporal requested a review from a team as a code owner June 5, 2026 19:30

@yycptt yycptt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good from chasm framework side. Please still take a look at the comments.

attrs chasm.TaskAttributes,
_ *schedulerpb.BackfillerTask,
) (bool, error) {
if attrs.IsImmediate() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does that mean we have an issue today?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nvm, immediate pure tasks will only run once today.
but agree the validation logic should invalidate the task after execution regardless if the pure task is immediate or not.

I will let scheduler crew review this part.

Comment thread chasm/errors.go
Comment on lines +37 to +45
TaskType string
TaskTypeID uint32
Archetype string
ArchetypeID ArchetypeID
ComponentPath []string
EncodedComponentPath string
ScheduledTime time.Time
Destination string
Immediate bool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we just embed TaskNotInvalidatedDetails here?

Comment thread chasm/errors.go
Comment on lines +67 to +68
ScheduledTime time.Time
Destination string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Reuse TaskAttributes?

Comment thread chasm/errors.go
return fmt.Sprintf(
"CHASM %s task remained valid after successful execution: %s",
e.TaskKind,
e.buildLogicalTaskReport(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally error messages should be fixed (so that filtering is easy) and any dynamic parts can be tags.

I think here you can either do the logging with additional tags inside ExecutePureTask or define an error details/ error log tags interface on the task processing side and implement it here.

we may also want to log task & component state (truncated to some size limit) as well to help debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants